Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

extending the c++ interface for plotting the parametrization #141

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

ndehio
Copy link
Contributor

@ndehio ndehio commented Sep 22, 2020

The new c++ method plot_parametrization(const int n_sample) reimplements the python-function plot_parametrization() from the file toppra/parametrizer.py.

The function is tested in the test test_toppra_approach.cpp. Note, however, that the output is similar but not identical to the python example. Your feedback is welcome.

… plot parametrization similar to the toppra python interface
@ndehio
Copy link
Contributor Author

ndehio commented Sep 22, 2020

For the future: how can I apply your clang format?

@hungpham2511
Copy link
Owner

Hi @ndehio, thanks for the contribution. However, I do not suggest this approach for "debugging" because it can be quite limited. For instance, the script that you printed can only used to debug and nothing else. I think it is better to serialize only the object internal data into a file stream and save to disk, then write a single python script to visualize it. This serialized data stream can then be used for debugging, logging, etc..

Currently, this is implemented for the geometric path via the serialize/deserialize methods here

void PiecewisePolyPath::serialize(std::ostream &O) const {

For this feature, I propose to implement serialize/deserialize methods for ConstAccel instead. We can store the result of the most recent evaluation and serialize those altogether.

@ndehio
Copy link
Contributor Author

ndehio commented Sep 24, 2020

I see your point, it makes sense to stay with the serialization approach.
However, did you have a look at the generated plot? I think there is still a problem with the c++ implementation of the parametrization.

bool ConstAccel::plot_parametrization(const int n_sample) {
// reimplements the function plot_parametrization() from the file toppra/parametrizer.py
Vector _ss = this->m_gridpoints;
Vector _velocities = this->m_vsquared;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Vector _velocities = this->m_vsquared;
Vector _velocities = this->m_vsquared.cwiseSqrt();

@hungpham2511
Copy link
Owner

We have test cases that verify the consistency of the output parametrization between the python and C++ implementation, so that shouldn't be the cause.

There is 1 bug in your code, I have commented on it.

One reason why the results are different is that the input path is not the same as in the python example. You can try the following Python code instead

import toppra
import numpy as np

s_array = np.array([0, 1, 2])
wp_array = np.array([(0, 0), (1, 2), (2, 0)])
wpp_array = np.array([(0, 0), (1, 1), (0, 0)])
# path = toppra.SplineInterpolator(s_array, wp_array)

path = toppra.SimplePath(s_array, wp_array, wpp_array)

pc_vel = toppra.constraint.JointVelocityConstraint([[-1, 1], [-0.5, 0.5]])
pc_acc = toppra.constraint.JointAccelerationConstraint([[-0.05, 0.2], [-0.1, 0.3]])

instance = toppra.algorithm.TOPPRA([pc_vel, pc_acc], path)
instance.compute_parameterization(0, 0)

instance.problem_data.return_code
instance.problem_data.gridpoints  #doctest: +ELLIPSIS
instance.problem_data.sd_vec  #doctest: +ELLIPSIS

path_new = toppra.ParametrizeConstAccel(path, instance.problem_data.gridpoints, instance.problem_data.sd_vec)
path_new.plot_parametrization(show=True)

@hungpham2511
Copy link
Owner

Figure_1

Both python and cpp code should produce a figure similar to the one above.

@github-actions
Copy link

Stale pull request message

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants